Skip to content

Conversation

@HanKuanChen
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Han-Kuan Chen (HanKuanChen)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/109948.diff

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+37)
  • (added) llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-concat-vectors.ll (+36)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index bf822eb2c6eeb5..fe14b1991d5da2 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -5130,6 +5130,40 @@ static SDValue lowerVECTOR_SHUFFLEAsRotate(ShuffleVectorSDNode *SVN,
   return DAG.getBitcast(VT, Rotate);
 }
 
+// e.g.,
+// t10 = insert_subvector undef:v16i32, t4, Constant:i64<0>
+// vector_shuffle<0,1,2,3,0,1,2,3,0,1,2,3,0,1,2,3> t10, undef:v16i32
+// ->
+// concat_vectors t4, t4, t4, t4
+static SDValue
+lowerVECTOR_SHUFFLEAsCONCAT_VECTORS(ShuffleVectorSDNode *SVN, SelectionDAG &DAG,
+                                    const RISCVSubtarget &Subtarget) {
+  assert(SVN->getOperand(1).isUndef());
+  SDValue V1 = SVN->getOperand(0);
+  if (V1.getOpcode() != ISD::INSERT_SUBVECTOR)
+    return SDValue();
+  if (!V1.getOperand(0).isUndef())
+    return SDValue();
+  SDValue InsertValue = V1.getOperand(1);
+  MVT SubVecVT = InsertValue.getSimpleValueType();
+  unsigned SubVecNumElements = SubVecVT.getVectorNumElements();
+  uint64_t InsertIndex = cast<ConstantSDNode>(V1.getOperand(2))->getZExtValue();
+  if (InsertIndex != 0)
+    return SDValue();
+  ArrayRef<int> Mask = SVN->getMask();
+  if (Mask.size() % SubVecNumElements != 0)
+    return SDValue();
+  SmallVector<int> RepeatedPattern(
+      createSequentialMask(0, SubVecNumElements, 0));
+  // Check the Mask repeatedly uses the same subvector.
+  for (unsigned I = 0; I != Mask.size(); I += SubVecNumElements)
+    if (!Mask.slice(I, SubVecNumElements).equals(RepeatedPattern))
+      return SDValue();
+  SDLoc DL(SVN);
+  SmallVector<SDValue> Ops(Mask.size() / SubVecNumElements, InsertValue);
+  return DAG.getNode(ISD::CONCAT_VECTORS, DL, SVN->getSimpleValueType(0), Ops);
+}
+
 // If compiling with an exactly known VLEN, see if we can split a
 // shuffle on m2 or larger into a small number of m1 sized shuffles
 // which write each destination registers exactly once.
@@ -5432,6 +5466,9 @@ static SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG,
     if (SDValue V = lowerVECTOR_SHUFFLEAsRotate(SVN, DAG, Subtarget))
       return V;
 
+    if (SDValue V = lowerVECTOR_SHUFFLEAsCONCAT_VECTORS(SVN, DAG, Subtarget))
+      return V;
+
     if (VT.getScalarSizeInBits() == 8 &&
         any_of(Mask, [&](const auto &Idx) { return Idx > 255; })) {
       // On such a vector we're unable to use i8 as the index type.
diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-concat-vectors.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-concat-vectors.ll
new file mode 100644
index 00000000000000..e3a790afeb41dd
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-concat-vectors.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple riscv64 -mattr=+v -riscv-v-vector-bits-min=512 -verify-machineinstrs < %s | FileCheck %s
+
+define <16 x float> @test1(<8 x float> %0) {
+; CHECK-LABEL: test1:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    vmv1r.v v9, v8
+; CHECK-NEXT:    vsetivli zero, 16, e32, m1, ta, ma
+; CHECK-NEXT:    vslideup.vi v9, v8, 8
+; CHECK-NEXT:    vmv.v.v v8, v9
+; CHECK-NEXT:    ret
+entry:
+  %1 = call <16 x float> @llvm.vector.insert.v16f32.v8f32(<16 x float> poison, <8 x float> poison, i64 8)
+  %2 = call <16 x float> @llvm.vector.insert.v16f32.v8f32(<16 x float> %1, <8 x float> %0, i64 0)
+  %3 = shufflevector <16 x float> %2, <16 x float> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
+  ret <16 x float> %3
+}
+
+define <16 x i32> @test2(<4 x i32> %0) {
+; CHECK-LABEL: test2:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    vmv1r.v v9, v8
+; CHECK-NEXT:    vsetivli zero, 8, e32, mf2, ta, ma
+; CHECK-NEXT:    vslideup.vi v9, v8, 4
+; CHECK-NEXT:    vmv1r.v v8, v9
+; CHECK-NEXT:    vsetivli zero, 16, e32, m1, ta, ma
+; CHECK-NEXT:    vslideup.vi v8, v9, 8
+; CHECK-NEXT:    ret
+entry:
+  %1 = call <16 x i32> @llvm.vector.insert.v16i32.v4i32(<16 x i32> poison, <4 x i32> poison, i64 4)
+  %2 = call <16 x i32> @llvm.vector.insert.v16i32.v4i32(<16 x i32> %1, <4 x i32> poison, i64 8)
+  %3 = call <16 x i32> @llvm.vector.insert.v16i32.v4i32(<16 x i32> %2, <4 x i32> poison, i64 12)
+  %4 = call <16 x i32> @llvm.vector.insert.v16i32.v4i32(<16 x i32> %3, <4 x i32> %0, i64 0)
+  %5 = shufflevector <16 x i32> %4, <16 x i32> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 0, i32 1, i32 2, i32 3, i32 0, i32 1, i32 2, i32 3, i32 0, i32 1, i32 2, i32 3>
+  ret <16 x i32> %5
+}

Comment on lines 13 to 14
%1 = call <16 x float> @llvm.vector.insert.v16f32.v8f32(<16 x float> poison, <8 x float> poison, i64 8)
%2 = call <16 x float> @llvm.vector.insert.v16f32.v8f32(<16 x float> %1, <8 x float> %0, i64 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does inserting a poison subvector into a posion vector do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it does nothing. Remove it already.

SDValue InsertValue = V1.getOperand(1);
MVT SubVecVT = InsertValue.getSimpleValueType();
unsigned SubVecNumElements = SubVecVT.getVectorNumElements();
uint64_t InsertIndex = cast<ConstantSDNode>(V1.getOperand(2))->getZExtValue();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simplify the zero check and maybe move it beside the undef check

  if (!V1.getOperand(0).isUndef() || V1.getConstantOperandVal(2) != 0)
    return SDValue();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@HanKuanChen HanKuanChen requested a review from lukel97 September 25, 2024 16:09
Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR could use a small description of the transform but otherwise LGTM

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple riscv64 -mattr=+v -riscv-v-vector-bits-min=512 -verify-machineinstrs < %s | FileCheck %s

define <16 x float> @test1(<8 x float> %0) {
Copy link
Collaborator

@topperc topperc Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InstCombine already flattens both of these tests. Do you have other examples?

define <16 x float> @test1(<8 x float> %0) {
entry:
  %1 = shufflevector <8 x float> %0, <8 x float> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
  ret <16 x float> %1
}

define <16 x i32> @test2(<4 x i32> %0) {
entry:
  %1 = shufflevector <4 x i32> %0, <4 x i32> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 0, i32 1, i32 2, i32 3, i32 0, i32 1, i32 2, i32 3, i32 0, i32 1, i32 2, i32 3>
  ret <16 x i32> %1
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true. But I want to make sure INSERT_SUBVECTOR + VECTOR_SHUFFLE can be folded into CONCAT_VECTORS. SLP will generate the code like this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InstCombine should run after SLP. Do you have examples where InstCombine isn't cleaning it up? If you do, we should investigate InstCombine first.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No for now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also doesn't seem like a RISC-V specific optimization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that as well. It looks like GISel canonicalizes shuffles to concats: https://reviews.llvm.org/D69149

I didn't notice any codegen changes with this patch when building SPEC or the test suite so I presume that's InstCombine already taking care of this.

@HanKuanChen
Copy link
Contributor Author

Close it since instcombine can solve this.

@HanKuanChen HanKuanChen closed this Oct 5, 2024
@HanKuanChen HanKuanChen deleted the riscv-lower-vector_shuffle branch October 5, 2024 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants